feat(gax): add getSingleHeader to HttpHeadersUtils - #14137
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new utility method getFirstHeader in HttpHeadersUtils to retrieve the first string value of a header by case-insensitive name from a headers map, along with a helper method extractFirstString to handle different header value types (such as iterables and non-string objects). Comprehensive unit tests have also been added to cover various scenarios, including case-insensitivity, iterable values, null values, and non-existent headers. There are no review comments, and I have no feedback to provide.
whowes
changed the title
d1c96b2 to
b7b5130
Compare
b7b5130 to
ed5f3b7
Compare
| return null; | ||
| } | ||
|
|
||
| private static @Nullable String extractFirstString(@Nullable Object headerValue) { |
There was a problem hiding this comment.
Does the protocol specify that we always use the first value of the header value?
There was a problem hiding this comment.
I believe that the protocol's control responses are only single-value headers, so I don't think that's part of the specification.
Since the value in the headers map can be a list (I'm not super familiar with this quirk of the HTTP client myself but it's the same pattern as documented in this comment) I opted to only return the first header. I suppose to be more general we could instead have the utility here return all values and have downstream resumable upload code handle the multi-value case.
There was a problem hiding this comment.
If the protocol says single-value headers only, can we change the code to only support that?
If the backend does return a list of values in the future though, we may need to adjust the logic accordingly. I don't think always getting the first value is a good assumption to make at this moment.
There was a problem hiding this comment.
Switched this to getSingleHeader() instead that fails-fast if a multi-valued header is encountered.
ed5f3b7 to
8f97deb
Compare
8f97deb to
04857e5
Compare
04857e5 to
6c9c801
Compare
6c9c801 to
5703db2
Compare
e31fa27 to
9402bac
Compare
9402bac to
e9ec582
Compare
e9ec582 to
545bd73
Compare
545bd73 to
0b2a27e
Compare
0b2a27e to
36fcba2
Compare
36fcba2 to
0a47cb0
Compare
whowes
changed the title
| * @throws IllegalArgumentException if multiple values are present for the header | ||
| */ | ||
| public static @Nullable String getSingleHeader(Map<String, Object> headers, String name) { | ||
| for (Map.Entry<String, Object> entry : headers.entrySet()) { |
There was a problem hiding this comment.
nit: We can use stream and findFirst for better readability.
There was a problem hiding this comment.
Great suggestion; done.
| public static @Nullable String getSingleHeader(Map<String, Object> headers, String name) { | ||
| for (Map.Entry<String, Object> entry : headers.entrySet()) { | ||
| if (name.equalsIgnoreCase(entry.getKey())) { | ||
| return extractSingleString(entry.getValue()); |
There was a problem hiding this comment.
What if we just do entry.getValue() == null ? null : entry.getValue().toString()? I think we can use the String value directly.
There was a problem hiding this comment.
We can't use the String directly because HttpHeaders (from which the map originates) represents headers, even single-value ones, as a List<String>. If we used toString() directly and the scalar value were "foo", we'd get "[foo]".
Added a brief comment to extractSingleString() pointing out this quirk of HttpHeaders.
0a47cb0 to
c92b3d3
Compare
c92b3d3 to
4e378cb
Compare
4e378cb to
4d1c16f
Compare
4d1c16f to
11f54da
Compare
11f54da to
2fca609
Compare
|
|
2 participants
The GAX HTTP/JSON stack stores response headers in a Map<String, Object> where values may be Strings, numbers, Iterables, etc., and header names may vary in casing depending on the HTTP version. This method provides case-insensitive lookup and safely extracts a single value in a typesafe way when it exists, returning null if not and throwing an exception if multiple values exist.
This will be used in resumable upload support to retrieve protocol header values (e.g. X-Goog-Upload-URL, Location, and X-Goog-Upload-Status).